Global delay bug fix: check again credits under mutex #438
Conversation
db/write_controller.cc
Outdated
| @@ -148,7 +148,13 @@ uint64_t WriteController::GetDelay(SystemClock* clock, uint64_t num_bytes) { | |||
| // Refill every 1 ms | |||
| const uint64_t kMicrosPerRefill = 1000; | |||
|
|
|||
| std::lock_guard<std::mutex> lock(mu_); | |||
| std::lock_guard<std::mutex> lock(metrics_mu_); | |||
There was a problem hiding this comment.
the use of credit_in_bytes_ is not right as i can see it... as you wrote above
"// This function is no longer under db mutex since credit_in_bytes_ is atomic"
in the first check you does threat it as atomic and do compare_exchange_weak meaning you do change its value not under mutex
but after you take the lock , you changed its value not using atomic operation which is problematic
the lock actually doesnt protect from you from access credit_in_bytes_...
you need to access credit_in_bytes_ with atomic functions
There was a problem hiding this comment.
right, we'll simply take the mutex before to include the first place where credit_in_bytes_ can change so now only 1 thread can change credit_in_bytes_ simultaneously
Yuval-Ariel
force-pushed
the
global-bug-fix-credits
branch
from
92b1d8a to
9361724
Compare
while at it, rename and reorder some members
Yuval-Ariel
force-pushed
the
global-bug-fix-credits
branch
from
9361724 to
802c219
Compare
While at it, rename and reorder some members
While at it, rename and reorder some members
While at it, rename and reorder some members
While at it, rename and reorder some members
While at it, rename and reorder some members
i encountered a bug while running tests for the dirty memory connection to the global write controller and the bug is as follows:
during WriteController::GetDelay , we check if there are enough credits and return if there are, here:
if there arent enough credits, we proceed under the mutex but dont check again the condition above which might have changed from a different thread.
this PR adds this additional check and while at it, rename and reorder some members.